@j4james, this is my proposal.

OK, this is much better than what I had. I ended up having to extend the OutputCellView and TextBufferCellIterator which is the part I hated the most (I need to look at the code again to see what made me think that was necessary). I was also concerned about the renderer having to draw the grid one character at a time, and wondering whether we might be able to optimise that a bit. But having that as a special case for wide characters does at least limit when that happens, so that's not too bad either.

I'm busy geeking out over the upcoming SpaceX launch at the moment, so I'll try and look at this in more detail later, but my initial impression is very positive.

I hate that this is possible. 😄

#6224 probably needs separate "line drawing runs" ...
that actually makes this way easier.

I hate that this is possible. 😄

#6224 probably needs separate "line drawing runs" ...
that actually makes this way easier.

Agreed. Each thing should probably be its own pass. If it can be efficient enough.

I've had a chance to run some more tests now, and I'm not sure this is working as well as I had thought. I have a little test case that tries to render every combination of grid flag around a wide character, and that doesn't seem to work with this PR. This is my test code:

#include <windows.h>
#include <stdio.h>

void test(WORD attr)
{
  HANDLE handle = GetStdHandle(STD_OUTPUT_HANDLE);
  SetConsoleTextAttribute(handle, 0x07);
  printf(" ");
  SetConsoleTextAttribute(handle, 0x07 | attr);
  printf("\xE5\xA4\xA7");
  SetConsoleTextAttribute(handle, 0x07);
}

void main()
{
  SetConsoleOutputCP(CP_UTF8);
  printf("\n");
  test(COMMON_LVB_GRID_LVERTICAL);
  test(COMMON_LVB_GRID_HORIZONTAL);
  test(COMMON_LVB_GRID_RVERTICAL);
  test(COMMON_LVB_UNDERSCORE);
  test(COMMON_LVB_UNDERSCORE | COMMON_LVB_GRID_LVERTICAL);
  test(COMMON_LVB_GRID_LVERTICAL | COMMON_LVB_GRID_HORIZONTAL);
  test(COMMON_LVB_GRID_HORIZONTAL | COMMON_LVB_GRID_RVERTICAL);
  test(COMMON_LVB_GRID_RVERTICAL | COMMON_LVB_UNDERSCORE);
  test(COMMON_LVB_UNDERSCORE | COMMON_LVB_GRID_HORIZONTAL);
  test(COMMON_LVB_GRID_LVERTICAL | COMMON_LVB_GRID_RVERTICAL);
  test(COMMON_LVB_GRID_LVERTICAL | COMMON_LVB_GRID_RVERTICAL | COMMON_LVB_UNDERSCORE);
  test(COMMON_LVB_GRID_LVERTICAL | COMMON_LVB_GRID_RVERTICAL | COMMON_LVB_GRID_HORIZONTAL);
  test(COMMON_LVB_GRID_LVERTICAL | COMMON_LVB_GRID_RVERTICAL | COMMON_LVB_GRID_HORIZONTAL | COMMON_LVB_UNDERSCORE);
  printf("\n");
}

This is what I expected it to look like (more or less - not sure about the center grid lines, but I think that's a separate issue):

This is what I'm seeing with this PR:

I'm hoping maybe I've just screwed up the build somehow. Will do some more digging.

One other thing I wanted to mention. It's not essential for this PR, but I thought while you're looking at this grid code, it might be a good time to get rid of COMMON_LVB_GRID_SINGLEFLAG. As far I could make out, it doesn't actually serve any purpose, and that would free up another bit for us to use in the attributes.

大 is

Conhost v1 with the scratch code:

Repaired OpenConsole on commit e1c757f:

One other thing I wanted to mention. It's not essential for this PR, but I thought while you're looking at this grid code, it might be a good time to get rid of COMMON_LVB_GRID_SINGLEFLAG. As far I could make out, it doesn't actually serve any purpose, and that would free up another bit for us to use in the attributes.

I went and read the code for this from conhostv1.

COMMON_LVB_GRID_SINGLEFLAG literally indicated to the IME output-buffer writer code that it needed to remove RVERTICAL from the left half of a two column character and LVERTICAL from the right half of a two column character. You know. Instead of determining that some other way and just stripping it out or only applying it to half the character.

So yeah, I think it can go away and/or be repurposed.

I believe this is good for merge. It restores the behavior to match v1 conhost in both through-the-API and IME cases.

Hello @DHowett!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

For the record this looked good to me too. Your solution turned out much simpler than what I thought would be required. 👍

🎉Windows Terminal Preview v1.1.1671.0 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes
• Store Download